Skip to content

fix(firestore): serialize session state as JSON to avoid reserved field name errors#5549

Open
nileshpatil6 wants to merge 2 commits intogoogle:mainfrom
nileshpatil6:fix/firestore-reserved-state-keys
Open

fix(firestore): serialize session state as JSON to avoid reserved field name errors#5549
nileshpatil6 wants to merge 2 commits intogoogle:mainfrom
nileshpatil6:fix/firestore-reserved-state-keys

Conversation

@nileshpatil6
Copy link
Copy Markdown

@nileshpatil6 nileshpatil6 commented Apr 29, 2026

Fixes #5539

Problem

`FirestoreSessionService` stores session state as a raw Firestore map, writing each state key as a direct document field path. Firestore rejects field names matching `.*` with:

```
400 field name 'session_metadata' is reserved
```

The ADK Developer UI creates sessions with `session_metadata` in the initial state (used to store display names). This makes the UI incompatible with `FirestoreSessionService` -- the first chat message always fails with `500 Internal Server Error`.

Fix

Serialize the session state dict as a JSON string before writing to Firestore, and deserialize on read. This avoids Firestore field name restrictions entirely since the entire state is stored as a single opaque string field.

Changes:

  • `create_session`: write `"state": json.dumps(session_state)` instead of `"state": session_state`
  • `get_session`: read back with `json.loads(raw_state) if isinstance(raw_state, str) else raw_state` for backward compat with existing sessions stored in old map format
  • `list_sessions`: same backward-compat read
  • `append_event`: write updated state as `json.dumps(session_only_state)`

Backward Compatibility

Existing sessions stored with the old dict format continue to work. The read path checks whether the stored value is a string (new format) or dict (old format) and handles both.

Why JSON string vs map

Firestore's `.*` restriction applies to all field paths including nested map sub-fields, so there is no way to store `session_metadata` as a Firestore map key at any level. Serializing the whole state as a JSON string sidesteps the restriction completely without any key escaping logic.

Testing Plan

Reproducing the original bug

The issue reporter's minimal repro already confirms the failure path. To verify locally before this fix:

```python
import asyncio
from google.adk.integrations.firestore.firestore_session_service import FirestoreSessionService
from google.cloud import firestore

async def main():
service = FirestoreSessionService(client=firestore.AsyncClient(), root_collection="adk-session")
await service.create_session(
app_name="test_app",
user_id="test_user",
session_id="test_session",
state={"session_metadata": {"displayName": "hello"}},
)

asyncio.run(main())

Before fix: raises google.api_core.exceptions.InvalidArgument: 400 field name 'session_metadata' is reserved

After fix: succeeds

```

What I verified manually

  1. `create_session` with `session_metadata` in state writes successfully to Firestore
  2. `get_session` on the same session returns state with `session_metadata` intact
  3. `append_event` that updates session state (e.g. sets a new key) writes and reads back correctly
  4. Backward compat: sessions created before this fix (state stored as a Firestore map with non-reserved keys) still load correctly because the read path falls back to treating the field as a dict when it is not a string
  5. ADK Developer UI flow works end-to-end: open UI, select agent backed by Firestore, send first message, session is created without 500 error

Unit test coverage

The existing unit tests in `tests/unittests/integrations/firestore/test_firestore_session_service.py` cover create/get/append_event for normal state keys. Running those tests after this change all pass (the mock Firestore client stores and returns the JSON string transparently).

A specific regression test for the reserved-key case would look like:

```python
async def test_create_session_with_reserved_key(firestore_session_service):
session = await firestore_session_service.create_session(
app_name="test_app",
user_id="test_user",
state={"session_metadata": {"displayName": "My Session"}},
)
assert session.state["session_metadata"]["displayName"] == "My Session"

loaded = await firestore_session_service.get_session(
    app_name="test_app",
    user_id="test_user",
    session_id=session.id,
)
assert loaded.state["__session_metadata__"]["displayName"] == "My Session"

```

Happy to add this as a formal test to the test file if that would help get this through review faster.

@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label Apr 29, 2026
@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented Apr 29, 2026

Response from ADK Triaging Agent

Hello @nileshpatil6, thank you for creating this PR!

Could you please include a testing plan section in your PR to describe how you tested this change? This will help reviewers to review your PR more efficiently. Thanks!

@rohityan rohityan self-assigned this Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ADK Developer UI sends Firestore-reserved __session_metadata__ state key

3 participants